Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[asic_sensors] Generate the asic_sensors polling configuration based on the platform.json #20826

Merged
merged 10 commits into from
Dec 24, 2024

Conversation

mlok-nokia
Copy link
Contributor

@mlok-nokia mlok-nokia commented Nov 15, 2024

Why I did it

For any platform which supports the asic_sensors pulling, it requires the configuration files (config_db#.json) contain the following configuration to trigger the system to poll the ASIC sensors temperature. Fixes https://github.com/Nokia-ION/ndk/issues/48
Adding Yang model support to fixes #20633

Work item tracking
  • Microsoft ADO (number only):

How I did it

  • Add new module src/sonic-config-engine/asic_sensors_config.py with function get_asic_sensors_config(). This function checks if device data platform.json contains the following configuration, it will generate the asic_sensors pulling configuration.
$ cat platform.json
{
...
...
...
   "asic_sensors": {
        "poll_interval": "10",
        "poll_admin_status": "enable"
    }
}
  • Modify the script sonic-cfggen platform option "-H" to call the function get_asic_sensor_config() to generate the asic_sensors pulling configuration.
  • Added new UT test_asic_sensors_config() to test_cfggen.py to test related the implementation.

Notice: For all platforms which support the asic_sensors polling requires to add the following definition to the device data platform.json file

$ cat platform.json
{
...
...
...
   "asic_sensors": {
        "poll_interval": "10",
        "poll_admin_status": "enable"
    }
}

Also add Yang Model support the ASIC_SENSORS configuration. INcluding YangModel UT.

How to verify it

  1. Running the new image on the platform which supports the ASIC_SENSORS
  2. Execute the CLI command "sonic-cfggen -H --print-data", the following code will be generated
{
   "ASIC_SENSORS": {
        "ASIC_SENSORS_POLLER_INTERVAL": {
            "interval": "10"
        },
        "ASIC_SENSORS_POLLER_STATUS": {
            "admin_status": "enable"
        }
    }
}

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202405

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@mlok-nokia mlok-nokia marked this pull request as ready for review November 15, 2024 22:44
@mlok-nokia mlok-nokia force-pushed the asic_sensor_pulling_config branch 2 times, most recently from fba67c0 to b26025d Compare November 18, 2024 19:05
@mlok-nokia mlok-nokia changed the title [asic_sensors] Generate the aisc_sensors pulling configuration based on the platform.json [asic_sensors] Generate the asic_sensors polling configuration based on the platform.json Nov 18, 2024
@mlok-nokia
Copy link
Contributor Author

@judyjoseph Please review this asic-sensors PR. Thanks

@rlhui rlhui requested a review from judyjoseph November 20, 2024 18:15
@mlok-nokia mlok-nokia changed the title [asic_sensors] Generate the asic_sensors polling configuration based on the platform.json [asic_sensors] Generate the asic_sensors polling configuration based on the platform.json and Yang Model supports Nov 21, 2024
@mlok-nokia mlok-nokia force-pushed the asic_sensor_pulling_config branch 2 times, most recently from 7c56970 to f991ffc Compare November 21, 2024 22:10
@mlok-nokia
Copy link
Contributor Author

@arlakshm This Includes the Yang Model for ASIC_SENSORS configuration. Please review it

@judyjoseph
Copy link
Contributor

Minor comments LGTM

@judyjoseph
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm
Copy link
Contributor

/Azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm
Copy link
Contributor

@mlok-nokia, can you divide this PR in two, one for the config_db related change and one for the platform related change

@mlok-nokia
Copy link
Contributor Author

mlok-nokia commented Dec 1, 2024

@mlok-nokia, can you divide this PR in two, one for the config_db related change and one for the platform related change
Platform related change is moved to PR - #20979. Please review it

@arlakshm
Copy link
Contributor

arlakshm commented Dec 2, 2024

/Azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

judyjoseph
judyjoseph previously approved these changes Dec 2, 2024
@mlok-nokia mlok-nokia changed the title [asic_sensors] Generate the asic_sensors polling configuration based on the platform.json and Yang Model supports [asic_sensors] Generate the asic_sensors polling configuration based on the platform.json Dec 19, 2024
@mlok-nokia
Copy link
Contributor Author

@mlok-nokia Could you please create a PR for sonic-yang-models? Modifying the YANG models should not introduce any elastic test errors.

You mean to move the Yang Model change to a different PR?

@mlok-nokia yes, please create a separate PR, current PR have some issue in KVM setup, fixing it would take some time, but asic sensors is already existing in config db, so the db_migrator would raise exception when doing config reload in physical testbed, example:

2024 Dec 16 06:02:45.075310 xxx ERR config_validator.py: Tables without yang models: {'ASIC_SENSORS': {'ASIC_SENSORS_POLLER_INTERVAL': {'interval': '10'}, 'ASIC_SENSORS_POLLER_STATUS': {'admin_status': 'enable'}}}
2024 Dec 16 06:02:45.098418 xxx ERR db_migrator: Caught exception: Yang validation failed

Thanks. Done.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mlok-nokia mlok-nokia force-pushed the asic_sensor_pulling_config branch from f068317 to 9e54124 Compare December 19, 2024 19:48
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mlok-nokia
Copy link
Contributor Author

@judyjoseph @arlakshm and @xwjiang-ms VS test failure have been fixed. Please review it again. Thanks.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@judyjoseph
Copy link
Contributor

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rlhui rlhui merged commit 18efba1 into sonic-net:master Dec 24, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Need YANG model for ASIC_SENSORS
10 participants